Skip to content

(fix)build: disable proto rule generation, rely on checked-in .pb.go for go-code compatibility#293

Merged
roychying merged 3 commits into
uber:mainfrom
roychying:chenghan.ying/disable-proto-gazelle
Jul 2, 2026
Merged

(fix)build: disable proto rule generation, rely on checked-in .pb.go for go-code compatibility#293
roychying merged 3 commits into
uber:mainfrom
roychying:chenghan.ying/disable-proto-gazelle

Conversation

@roychying

@roychying roychying commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Why?

We want the smoothest DX for consuming the OSS submitqueue repo in go-code. Today go-code's go_repository uses build_file_generation = "on", so its Gazelle regenerates BUILD files for the vendored repo. This causes conflicts and mismatches, and every version bump risks new patching.

The root issue is that OSS carries proto types twice: as checked-in .pb.go in protopb/, and as Bazel-generated code via go_proto_library in proto/. Removing the Bazel proto rules in favor of the checked-in .pb.go fixes several things at once:

  • Eliminates duplicate proto definitions. The two generation paths share the same importpath, which caused the duplicate-importpath ambiguity and forced the gazelle:resolve workarounds. Collapsing to a single source removes that whole class of conflict.
  • Serves non-Bazel consumers. The Bazel-generated code only exists inside a Bazel build graph. Anyone consuming submitqueue via plain go build/go get (including OSS users) can only use the checked-in .pb.go, so that path must work regardless. The Bazel proto rules are a redundant second path that serves no one the checked-in files don't already serve.

What?

Disable Gazelle proto rule generation and remove all proto compilation rules, relying solely on checked-in .pb.go files.

  • Add # gazelle:proto disable_global to prevent Gazelle from regenerating proto rules on future make gazelle runs
  • Remove the gazelle:resolve directives that are no longer needed
  • Remove proto_library, go_proto_library, and alias go_library rules
  • Keep exports_files for make proto codegen (tool/proto/BUILD.bazel)

Test Plan

  • make build - pass
  • make test - 65/65 unit tests pass (integration/e2e tests fail due to Docker dependency, pre-existing)
  • bazel build //tool/proto:generated - proto codegen still works
  • make gazelle - zero warnings, no duplicate-importpath ambiguity
  • Validated in go-code with --override_repository pointing at this branch: all 71 targets build, all 20 tests pass across stovepipe, submitqueue/gateway, and submitqueue/orchestrator

Issue

@roychying roychying requested review from a team, behinddwalls and sbalabanov as code owners July 1, 2026 19:12
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


chenghan.ying seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@roychying roychying changed the title Chenghan.ying/disable proto gazelle (fix)build: disable proto rule generation, rely on checked-in .pb.go for go-code compatibility Jul 1, 2026
@roychying roychying added this pull request to the merge queue Jul 2, 2026
Merged via the queue into uber:main with commit bf31d60 Jul 2, 2026
14 of 15 checks passed
behinddwalls pushed a commit that referenced this pull request Jul 7, 2026
Adopt go_default_library naming convention for Bazel targets 

## Why?
The internal go-code monorepo consumes this repo via Bazel with
`build_file_generation = "off"` - it uses the checked-in `BUILD.bazel`
files **directly** instead of regenerating them. go-code's gazelle
enforces the`go_default_library` naming convention for all external
repos globally (`# gazelle:go_naming_convention_external
go_default_library`), so every dependency label it writes points at
`:go_default_library`. For go-code to reference this repo's targets
without per-import fixups, our targets must be reachable as
`:go_default_library`.

This complements the proto-disable change
(#293): together they let
go-code build this repo out of the box on Bazel 8.5.1 (which removed
`native.java_proto_library`, breaking `proto_library`).

## What?
- Add `# gazelle:go_naming_convention go_default_library` to the root
`BUILD.bazel` and regenerate target names via gazelle:
libraries/binaries ->`go_default_library`, tests -> `go_default_test`;
external refs normalize to `:go_default_library`.
- Manually migrate hand-authored, custom-named test targets that gazelle
cannot rename across a convention change:
- 9 integration/e2e `go_test` targets - `go_default_test` (`data`/`tags`
preserved once the name matches the convention),
- remove the redundant `platform/consumer:consumer_test` (gazelle now
emits `go_default_test` for that package),
  - point the `Makefile` integration-test targets at `:go_default_test`.
- Drop redundant `# keep`-pinned short-name `protopb` deps - under the
new convention the `:go_default_library` equivalents are emitted beside
them, so the short-name `:protopb` labels are dangling.

## Test Plan
- `make gazelle` - clean and idempotent (no duplicate-rule errors).
- `make build` -  223 targets built.
- `make test` -  65/65 unit tests pass.
- Verified downstream consumption from go-code with
`build_file_generation="off"`
  by overriding the module at this branch:
  bazel test //src/.../submitqueue/{gateway,orchestrator,stovepipe}/...
--override_repository=com_github_uber_submitqueue=/path/to/submitqueue
  -  71 targets build, 20/20 consumer tests pass.


## Issue
<!-- 
Link the issue here. 
- Use 'Closes #123' if this is the final fix. 
- Use 'Part of #123' or just '#123' if the feature is still in progress.
-->

Co-authored-by: chenghan.ying <chenghan.ying@uber.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants